-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[AArch64] Stop reserved registers from being saved in prolog/epilog #138448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-backend-aarch64 Author: None (yasuna-oribe) ChangesGCC's man page is clear on how -ffixed-reg must behave:
This implies prolog/epilog code also must not save/restore explicitly fixed registers, even when it is callee-saved. Some projects rely on this (GCC's) behavior. For example,
should not touch x28 outside of For riscv64, clang behaves the same as GCC, so I am inclined to believe this is indeed a bug. Fixes #111379. Full diff: https://github.com/llvm/llvm-project/pull/138448.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 78ac57e3e92a6..2d72f8757d7c0 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -3619,6 +3619,13 @@ void AArch64FrameLowering::determineCalleeSaves(MachineFunction &MF,
if (Reg == BasePointerReg)
SavedRegs.set(Reg);
+ // Don't save fixed registers specified with -ffixed-reg.
+ if (AArch64::GPR64RegClass.contains(Reg) &&
+ RegInfo->isReservedReg(MF, Reg)) {
+ SavedRegs.reset(Reg);
+ continue;
+ }
+
bool RegUsed = SavedRegs.test(Reg);
unsigned PairedReg = AArch64::NoRegister;
const bool RegIsGPR64 = AArch64::GPR64RegClass.contains(Reg);
|
This comment was marked as resolved.
This comment was marked as resolved.
Yep, tests succeeded now. I'm sorry for the force push, I just noticed I'm supposed to use |
…/epilog GCC's man page is clear on how -ffixed-reg must behave: ``` Treat the register named reg as a fixed register; generated code should never refer to it (except perhaps as a stack pointer, frame pointer or in some other fixed role). ``` This implies prolog/epilog code also must not save/restore explicitly fixed registers, even when it is callee-saved. Some projects rely on this (GCC's) behavior. For example, ``` void f() { register uint64_t x28 asm("x28") = 0xee; asm volatile("" : "+r"(x28)); // avoid mov being eliminated } ``` should not touch x28 outside of `mov w28,#0xee`. For riscv64 clang behaves the same as GCC. Fixes llvm#111379.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Would you be willing to add some tests just to verify behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some tests? I have a feeling this has come up before and there were edge cases to worry about, but don't know the history.
I'd like to check with our GCC team about the behaviour of -ffixed particularly for a Callee-saved register. The GCC docs leave a lot of room for interpretation. At one level -ffixed-xN can be seen as "I'm globally reserving xN for the whole program" In which case it would make sense to exclude a registers from the callee saved set. An alternative interpretation is "I'm globally reserving xN for the functions in this translation unit" for example I am using binary library code that I don't control so I can't globally guarantee that xN is reserved. In that case it would make sense to still callee save the register, although it would mean that only temporary registers (like x18) can be used if we require no callee saves. |
GCC for AArch64 does seem to callee save these registers https://godbolt.org/z/3oj3dMb1P so it seems like the As expected it won't save if a temporary register like x9 is used instead of a callee save register. |
@smithp35, do you mean it doesn't? (https://godbolt.org/z/6PWT4WP1h) If I'm not mistaken, I don't see x28 (callee-saved reg) being saved or restored anywhere in the GCC assembly, regardless if it's a local or global variable. This pull request is specifically for that GCC behavior, which llvm's riscv64 target follows (https://godbolt.org/z/cxeqnc6hT). @nasherm @davemgreen I've added the tests (llvm/test/CodeGen/AArch64/reserveXreg.ll). |
GCC's documentation is clear on how -ffixed-reg must behave:
This implies prolog/epilog code also must not save/restore explicitly fixed registers, even when it is callee-saved. Some projects rely on this (GCC's) behavior.
For example,
should not touch x28 outside of
mov w28,#0xee
.For riscv64, clang behaves the same as GCC, so I am inclined to believe this is indeed a bug.
Fixes #111379.